Skip to content

Reliably disable ANSI escaping in command output when output is redirected; add support for NO_COLOR; fix inconsistencies - #480

Merged
nblumhardt merged 3 commits into
datalust:devfrom
nblumhardt-ro:ansi-escapes
Jul 29, 2026
Merged

Reliably disable ANSI escaping in command output when output is redirected; add support for NO_COLOR; fix inconsistencies#480
nblumhardt merged 3 commits into
datalust:devfrom
nblumhardt-ro:ansi-escapes

Conversation

@nblumhardt-ro

Copy link
Copy Markdown

Consolidation of output formatting into a single OutputFormat type has been an ongoing process. I started chasing an ANSI escaping bug (ANSI escape sequences appearing in redirected output) and decided to try consolidating more in the process, which came out reasonably nicely.

seqcli print now relies on OutputFormat instead of duplicated formatting logic. Handling of colour-related flags, settings, and environment variables is now centralized into the OutputFormat constructor.

Getting rid of Serilog.Expressions/Serilog console theming in favour of Seq.Syntax and expression templates would be a worthwhile follow-up.

Nicholas Blumhardt added 3 commits July 28, 2026 11:51
// Serilog's console sink applies the `NO_COLOR` convention itself, unconditionally, overriding whatever
// theme it's passed. When `--force-color` has opted back out of `NO_COLOR`, the variable is cleared (for
// this process only) so that the sink can't undo the decision made here.
if (!NoColor && noColorSetInEnvironment)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this worth the quirky workaround?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we migrate away from using ConsoleTheme this will (thankfully) go away; until then it does at least mean that --force-color works consisently for all commands.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would it take to move away from ConsoleTheme?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not a huge effort, mostly digging into why we haven't been able to do it previously :-)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give it a shot tomorrow; want me to fold it in here, or send a follow-up?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can do it as a follow-up, because this is still 90% of what we want either way?

@nblumhardt
nblumhardt merged commit 4162fdd into datalust:dev Jul 29, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants